Conversation
mroloux
requested review from
PabloNeirotti,
bverbeken,
Copilot,
mortendevold,
schaloner and
sdivelbiss
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix a release issue where the latest release (83.2.0) was published with incomplete code by reverting changes from PR #127. The changes revert from using uv version command to a simpler string replacement approach for version bumping, and remove the setuptools packages configuration from pyproject.toml.
- Reverts release.mjs to use manual string replacement instead of
uv versioncommand - Bumps version from 83.2.0 to 83.3.0 in pyproject.toml and uv.lock
- Removes [tool.setuptools] packages configuration from pyproject.toml
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| release.mjs | Reverts to simpler replaceInFile implementation that manually updates version strings instead of using uv CLI |
| pyproject.toml | Bumps version to 83.3.0 and removes setuptools package configuration |
| uv.lock | Updates version reference to 83.3.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async function replaceInFile(filename, latestVersion, nextVersion) { | ||
| return await fs.readFile(filename, 'utf8') | ||
| .then(text => { | ||
| if (text.indexOf(latestVersion) < 0) { | ||
| throw new Error('Not the correct version. Could not find ' + latestVersion + ' in ' + filename) | ||
| } | ||
| return text | ||
| }) | ||
| .then(text => text.replace(latestVersion, nextVersion)) |
There was a problem hiding this comment.
The replaceInFile function has incorrect parameter naming. The parameters are named 'latestVersion' and 'nextVersion', but they're being used as generic 'old text' and 'new text' strings. The function performs a simple string replacement of the second parameter with the third parameter, so the parameter names should be more generic like 'oldText' and 'newText' or 'searchText' and 'replacementText' to avoid confusion.
| async function replaceInFile(filename, latestVersion, nextVersion) { | |
| return await fs.readFile(filename, 'utf8') | |
| .then(text => { | |
| if (text.indexOf(latestVersion) < 0) { | |
| throw new Error('Not the correct version. Could not find ' + latestVersion + ' in ' + filename) | |
| } | |
| return text | |
| }) | |
| .then(text => text.replace(latestVersion, nextVersion)) | |
| async function replaceInFile(filename, searchText, replacementText) { | |
| return await fs.readFile(filename, 'utf8') | |
| .then(text => { | |
| if (text.indexOf(searchText) < 0) { | |
| throw new Error('Not the correct version. Could not find ' + searchText + ' in ' + filename) | |
| } | |
| return text | |
| }) | |
| .then(text => text.replace(searchText, replacementText)) |
| async function replaceInFile(filename, latestVersion, nextVersion) { | ||
| return await fs.readFile(filename, 'utf8') | ||
| .then(text => { | ||
| if (text.indexOf(latestVersion) < 0) { | ||
| throw new Error('Not the correct version. Could not find ' + latestVersion + ' in ' + filename) | ||
| } | ||
| return text | ||
| }) | ||
| .then(text => text.replace(latestVersion, nextVersion)) |
There was a problem hiding this comment.
The error message incorrectly refers to the parameter as 'latestVersion' when it's actually the second parameter passed to the function. The error message should be more accurate to reflect that it's checking for the presence of the search text. For clarity, consider updating the error message to match the actual purpose of the check.
| async function replaceInFile(filename, latestVersion, nextVersion) { | |
| return await fs.readFile(filename, 'utf8') | |
| .then(text => { | |
| if (text.indexOf(latestVersion) < 0) { | |
| throw new Error('Not the correct version. Could not find ' + latestVersion + ' in ' + filename) | |
| } | |
| return text | |
| }) | |
| .then(text => text.replace(latestVersion, nextVersion)) | |
| async function replaceInFile(filename, searchText, nextVersion) { | |
| return await fs.readFile(filename, 'utf8') | |
| .then(text => { | |
| if (text.indexOf(searchText) < 0) { | |
| throw new Error('Could not find search text "' + searchText + '" in ' + filename) | |
| } | |
| return text | |
| }) | |
| .then(text => text.replace(searchText, nextVersion)) |
|
|
||
| [tool.setuptools] | ||
| packages = ["seatsio"] | ||
| build-backend = "setuptools.build_meta" No newline at end of file |
There was a problem hiding this comment.
The removal of the [tool.setuptools] packages configuration is likely the root cause of the incomplete release. Without explicitly specifying packages = ["seatsio"], setuptools won't know which package to include in the distribution, resulting in an empty or incomplete package. This configuration should be kept to ensure the seatsio package is properly included in the release.
| build-backend = "setuptools.build_meta" | |
| build-backend = "setuptools.build_meta" | |
| [tool.setuptools] | |
| packages = ["seatsio"] |
Temp fix for #130. In our latest release, most of the code is not present:
So I'm revering the code that bumps uv.lock for now.